Review — PR #843 (round 1)

Confidentiality: clean. The three changesets, the new packages/harness-desktop/README.md, and
every new comment name only Sapiom, Claude Code / Codex, npm and the public registry. No customer,
codename, business arrangement, private host, or incident reference. No hardcoded internal defaults
(the only URL is https://registry.npmjs.org/). Nothing in the diff discloses a vulnerability.

Findings

1. agent-versions/ grows without bound and is never reclaimed — MEDIUM-HIGH

packages/harness-desktop/src/main/agent-updates.ts only ever creates directories. Grepping the
package, agent-versions appears in exactly two places: boot.ts:346 (the root) and the README.
Nothing deletes anything, ever. Three leaks compound:

• Superseded prefixes. Every accepted update mints <kind>/<version>-<uuid>/ and leaves the
  previous one forever. agent-updates.test.ts:691 ("retains older prefixes when selecting a new
  version") pins this as intended.
• Failed prefixes. mkdir(prefix) happens before install; on failure, wrong version, or abort
  the partially-populated tree stays. The retains the working executable…after %s test literally
  writes partial-download into the prefix and never asserts it is cleaned up.
• Orphaned selectors. active-<uuid>.json is written with flag: "wx", then
  options.signal?.throwIfAborted() runs before rename (agent-updates.ts, publish block). Abort
  there — which is exactly what quitting during setup does — leaks the temp file permanently.

Failure scenario: a user on a machine where Codex moved 0.100.0 → 0.153.4 (the PR's own validation)
accumulates one full CLI install per accepted update. @anthropic-ai/claude-code and @openai/codex
are each tens to 100+ MB installed, so this is hundreds of MB to GBs silently parked in userData,
with no eviction path and no uninstall hook.

Fix is cheap and safe: GC at the top of ensureAgentUpdates — remove active-*.json temp files and
any <kind>/* directory matching INSTALL_DIRECTORY that is not the one named by active.json.
Doing it at boot is safe precisely because no PTY from the previous launch survives, so the
"processes still using them" argument in the README does not apply across restarts.

2. Every launch blocks on the network and a possible install, with no in-app opt-out — MEDIUM

ensureAgentUpdates is awaited in boot() before doctor and before the main window
(boot.ts:344), and iterates providers sequentially. Worst case per provider: 5s external probe +
5s managed probe + 5s registry + 90s install (installAgentVersion) + 5s verify probe ≈ 110s,
so ~3.5 min added to launch for a user whose two CLIs are both stale. The escape hatches are
--dev, --smoke, and SAPIOM_DISABLE_AGENT_UPDATES=1 — none reachable by a user who double-clicks
the app, which is this package's entire target audience. There is no persisted setting and no
"skip" affordance on the setup window; the only user-side cancel is quitting.

Either move the update off the boot critical path (select on the next launch, which the immutable
active.json design already supports), or add a Settings toggle alongside the app-updater
preferences so this is a user decision rather than an env var.

3. Agent package names are now a second source of truth — LOW-MEDIUM

managed-agent.ts hardcodes @anthropic-ai/claude-code and @openai/codex in AGENT_PACKAGES,
while agent-install.ts:88 exists specifically so the desktop installer does not hardcode them
("Parsing the constant instead of hardcoding the name keeps the desktop installer from drifting
from what the CLI itself tells users to run"), and boot.ts already imports both
CLAUDE_INSTALL_COMMAND and CODEX_INSTALL_COMMAND. If the harness ever changes an install
command, the setup path follows it and the update path silently keeps installing the old package.
Derive AGENT_PACKAGES[*].package via packageSpecFromInstallCommand() from the two constants.

Notes (not findings)

• The changeset levels are consistent with this repo's actual convention (minor reserved for
  breaking-for-embedders while @sapiom/harness is pre-1.0; additive surface ships as patch), so
  patch for the new createCodexAdapter export and the binaryArgs/binaryEnv adapter options is
  correct here. Both new option fields are load-bearing for the desktop host and carry JSDoc.
• @sapiom/harness-desktop is private: true, so the new README and the agent-versions behavior
  never reach an npm tarball. No files/exports/engines changes; no new runtime dependencies.
• The verify-then-rename publish order, the INSTALL_DIRECTORY / startsWith(version-) selector
  validation, the PID-reuse guard in windowsUpdateTree, and the windowsHide: true on every new
  child_process call are all correct and well covered.

Verdict: Request changes — the unbounded agent-versions/ growth (finding 1) should be fixed
before merge; findings 2 and 3 are product/maintenance calls the author can decide.

Review — PR #843 (round 2, follow-up)

Delta reviewed: 06670449 → 4d594686 (3 commits: failed-attempt cleanup, process-group
ownership + ELECTRON_RUN_AS_NODE containment, CI timeout). Confidentiality: clean — the two
changeset edits, the new desktop-managed-installs.md, the README lines and the new comments name
only Sapiom/Studio, Claude Code, Codex, Electron and npm. No new runtime deps, no new public
surface, no hardcoded internal defaults.

Earlier findings the push did NOT fix

• cleanup HTTP SDKs #1 (disk growth) — partially fixed, core still open. The finally block in
  agent-updates.ts:180 now removes this attempt's unpublishedPrefix/unpublishedSelector, which
  closes the failed-install and orphaned-active-*.json leaks. Superseded prefixes are still
  never reclaimed — agent-updates.test.ts:226 still pins retention and README.md now states
  "automatic eviction is not implemented" — so the recurring, unbounded part of the finding stands:
  one full CLI install (tens to 100+ MB) per accepted update, forever, in userData.
• cleanup HTTP SDKs #1 caveat. Cleanup is best-effort (rm(...).catch(() => {})), and the quit-during-install case
  is exactly when npm may still be writing into the prefix. With no boot-time GC, any cleanup that
  loses that race leaves the partial tree permanently.
• update SDKs to use payment protocol data #2 (boot blocks on network/install; no in-app opt-out) — unchanged. Still --dev, --smoke,
  SAPIOM_DISABLE_AGENT_UPDATES=1 only.
• langchain SDK config cleanup #3 (package names hardcoded in AGENT_PACKAGES) — unchanged, managed-agent.ts:4.

New finding

• LOW — timeout no longer kills detached descendants. agent-update-process.ts:203 now skips
  process.kill(-pid) whenever exitedAt !== undefined. The PID-reuse motivation is sound, but a
  descendant that closes its stdio (an npm lifecycle script that daemonizes) lets the supervisor exit
  first, so the 90 s deadline and the quit path both terminate nothing — round-1 code killed the
  group. Worth a comment stating the tradeoff, or reap by pgid identity rather than skipping.

The --import data:…delete process.env.ELECTRON_RUN_AS_NODE injection checks out: the flag is
supplied explicitly everywhere it is still needed (boot.ts:577, shim-files.ts:32), and
managed-agent.test.ts plus the smoke.ts fixture verify non-leakage into the CLI and its children.
Nothing the earlier round asserted turned out to be wrong.

Verdict: Approve with comments — only finding #1's superseded-prefix growth still warrants a fix
before merge; the rest are the author's call.